-
-
Notifications
You must be signed in to change notification settings - Fork 201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make doxygenfunction more robust when matching parameters #723
Make doxygenfunction more robust when matching parameters #723
Conversation
Now updated with tests and more systematic stripping of information that doesn't contribute to overloading. |
@jakobandersen just curious: are the changes in this PR independent of those in #711? I believe I need both sets of patches for my use case, so I was wondering if I could just rebase one on the other and install locally. |
Yes, they should be. As they both include the commits of #698 it may be icky to do it as a rebase, but I just tried locally to merge the two branches, which should work without problems. |
Note, both of these PRs are on top of the changes introduced by jakobandersen#698. Links to PRs for future reference: - breathe-doc#698 - breathe-doc#711 - breathe-doc#723 Signed-off-by: Abrar Rahman Protyasha <[email protected]>
Note, both of these PRs are on top of the changes introduced by upstream PR breathe-doc#698. Links to PRs for future reference: - breathe-doc#698 - breathe-doc#711 - breathe-doc#723 Signed-off-by: Abrar Rahman Protyasha <[email protected]>
@jakobandersen - are you happy for this to be merged? Also should I make you a collaborator on the repo? I assume that has come up before but I'm surprised that you're not. |
Indeed, this is an icky case, and in the end a matter of style. Though, this is the rendering currently defined in Sphinx, where I generally put the declarators with the name, e.g., |
In principle it is ready to merge, but as it includes #698 we should get that one fixed first. Though, I'd like to rebase it so the CI can run through. You are welcome to make me collaborator. I'll probably only merge non-trivial things my self and keep the previous workflow with at least one reviewer for the larger things. |
4c0b7a8
to
b3ec822
Compare
Note to self, need update after the parsing fix has been merged. |
b3ec822
to
341afb7
Compare
(This depends on / includes #698)
Fixes the basics of #722, and all(?) other cases where information does not contribute to overload resolution. See
examples/specific/cpp_function_lookup.h
for examples (https://github.com/michaeljones/breathe/pull/723/files?file-filters%5B%5D=.h&file-filters%5B%5D=.py#diff-b4e4a1134077e09d357e73a9c4ca3cb49fec09a3264c88bc5119aeaa48990721).Things that may still fail:
decltype(expr)
which would make all but one overload valid. Currently the trailing return type is stripped. The syntax ofdoxygenfunction
needs to be expanded to properly handle this.doxygenfunction
needs to be expanded to properly handle this.int *p
vs.int p[]
). The user must use the same syntax as in the code. I'm not sure if it's worth making this work.